Skip to content

wolfsftp: reject symlink leaf in SFTP_RecvOpen#1014

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_4792
Open

wolfsftp: reject symlink leaf in SFTP_RecvOpen#1014
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_4792

Conversation

@yosuke-wolfssl

@yosuke-wolfssl yosuke-wolfssl commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Description

wolfSSH_SFTP_RecvOpen checked a target's type with stat() (follows symlinks) and then opened it with open() lacking O_NOFOLLOW. A symlink whose leaf points at a regular file passed the FILEATRB_PER_FILE type check, and open() then followed it to the target — a confused-deputy read/write of any file the daemon can reach, plus a TOCTOU race on the final path component.
Addressed by f_4792.

Changes

Harden the open sink, scoped to confined sessions so behavior is unchanged for unconfined servers (which follow symlinks by default):

  • RecvOpen now computes a confined flag via a new SFTP_SessionConfined() helper and, only when confined:
    • passes noFollow=1 to SFTP_GetAttributes, so POSIX uses lstat() and a symlink leaf is reported as FILEATRB_PER_LINK and rejected;
    • ORs O_NOFOLLOW into the open flags, closing the final-component TOCTOU window.
  • New portable WOLFSSH_O_NOFOLLOW macro in port.h (O_NOFOLLOW on POSIX where available, 0 no-op fallback elsewhere), guarded with #ifdef O_NOFOLLOW so POSIX targets lacking it degrade gracefully.
  • The confinement gate (trailing-separator-stripped default-path length > 1) is factored into a single SFTP_ConfinedPathLen() helper shared by GetAndCleanPath and SFTP_SessionConfined, so the two stay in sync.
  • New test for positive/negative cases

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jun 10, 2026
Copilot AI review requested due to automatic review settings June 10, 2026 01:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the SFTP server’s RecvOpen path against symlink-leaf confused-deputy and final-component TOCTOU issues by ensuring confined sessions do not follow symlinks when checking file type and opening files.

Changes:

  • Add a portable WOLFSSH_O_NOFOLLOW macro (with a safe 0 fallback) and apply it to confined SFTP opens.
  • Factor confinement-prefix length logic into SFTP_ConfinedPathLen() and use it consistently to gate confined-session behavior.
  • Extend the SFTP confinement test to include a “leaf is a symlink to an in-jail regular file” case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
wolfssh/port.h Introduces WOLFSSH_O_NOFOLLOW (mapped to O_NOFOLLOW where available, otherwise 0) to support safe flag OR’ing.
src/wolfsftp.c Adds confinement helpers and uses lstat-style attribute checks + O_NOFOLLOW when confined to reject symlink leaves and reduce TOCTOU risk.
tests/api.c Adds a confinement regression test ensuring opening a symlink leaf is rejected while opening the direct regular file still succeeds.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/api.c Outdated

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #1014

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread tests/api.c
Comment thread tests/api.c

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #1014

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread tests/api.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants